feat(walletconnect): walletconnect integration #2223

borngraced · 2024-09-16T19:11:09Z

This PR introduces the integration of WalletConnect into the Komodo DeFi Framework (KDF), enabling secure wallet connections for Cosmos and EVM-based chains. KDF acts as the DApp(in this PR), allowing users to initiate and manage transactions securely with external wallets e.g via Metamask.
Key changes include:

Implement multi-session handling for concurrent WalletConnect connections across accounts, integrates SQLite/IndexedDB for persistent session storage across devices, enhances relayer disconnection handling for smoother session management
Implemented WalletConnect coin activation for Tendermint and EVM.
Implemented Withdraw and Swap functionalities for Tendermint and EVM

https://specs.walletconnect.com/2.0/specs/clients/sign/
https://specs.walletconnect.com/2.0/specs/clients/core/pairing/
https://specs.walletconnect.com/2.0/specs/clients/core/crypto/
https://specs.walletconnect.com/2.0/specs/servers/relay/
https://docs.reown.com/advanced/multichain/rpc-reference/ethereum-rpc

Additional improvements include cleanup of unused dependencies, minor code refinements, and WASM compatibility

Updated deps:

aead: 0.5.2
aes-gcm: 0.10.3
anyhow: 1.0.89
proc-macro2: 1.0.86
quote: 1.0.37
bip39: 2.1.0
chacha20: 0.9.1
chacha20poly1305: 0.10.1
curve25519-dalek: 4.1.3
ctr: 0.9.2
ghash: 0.5.1
lock_api: 0.4.12
syn: 2.0.77
snow: 0.9.6
signature: 1.6.4
serde_json, serde, serde_derive: 1.0.210

Added deps:

Removed deps:

cipher: 0.4.4 / 0.3.0
aes: 0.8.3
libm: 0.2.7
platforms: 3.0.2

How to test using EVM coin e.g `ETH` (Native && WASM)

Start kdf (cargo run)
Create WalletConnect session connection
- Use the RPC below
- Copy the connection URL and either:
  - Paste directly in your wallet or
  - Generate QR code using QR Code Generator and scan with your wallet (e.g., MetaMask)

{
     "method": "wc_new_connection",
	"userpass": "{{ _.userpass }}",
	"mmrpc": "2.0",
	"params": {
		"namespaces": {
			"eip155": {
				"chains": ["eip155:1"]
				"methods": [
					"eth_sendTransaction",
					"eth_signTransaction",
					"personal_sign"
				],
				"events": [
					"accountsChanged",
					"chainChanged"
				]
			}
		}
	}
}

Activate ETH coin (set "priv_key_policy": "WalletConnect", in activation params).
Approve authentication request in your Wallet
Withdraw or trade.

Note: To add more eip155 chains, modify the chains array like this: ["eip155:1", "eip155:250"]

…ubscriptions

# Conflicts: # mm2src/coins/Cargo.toml # mm2src/coins_activation/src/eth_with_token_activation.rs # mm2src/mm2_core/src/mm_ctx.rs

mm2src/mm2_main/src/rpc.rs

shamardy · 2024-11-08T19:16:50Z

@onur-ozkan This PR requires your review on the tendermint code changes at least

laruh · 2024-11-12T08:39:22Z

@borngraced cargo lock conflict occurred

# Conflicts: # Cargo.lock

mm2src/coins_activation/src/tendermint_with_assets_activation.rs

onur-ozkan · 2024-11-13T11:46:22Z

mm2src/coins_activation/src/tendermint_with_assets_activation.rs

+    /// Activation via WalletConnect
+    WalletConnect,


Could you add some info about why we don't take public key similar to how it's done with WithPubkey variant?

Also, I'm curious why WithPubkey mode doesn’t work here. Doesn’t WalletConnect already provide the public key via an API? WithPubkey mode was intended to support any type of external wallet as it only requires the public key and doesn’t depend on the wallet’s internal logic.

WalletConnect doesn’t directly provide the public key as part of the initial session information. Instead, it only returns the account address, methods and the chains the user can interact with. To obtain the public key, a separate request is required for the specific account

https://specs.walletconnect.com/2.0/specs/clients/sign/rpc-methods#wc_sessionsettle
https://specs.walletconnect.com/2.0/specs/clients/sign/session-proposal

I would have done it in a way where some library/crate (or even GUI side) handles this and then utilizes with_pubkey on core KDF side to have single entrypoint for any external wallet, so the complexity remains constant for N external wallet types.

this is like dividing this implementation into two (manages connection from GUI while kdf manages request handling ) or leaving everything entirely to GUI which is not what we wanted from the beginning.

So the thing is whoever wishes to use any wallet pubkey via WalletConnect needs to make an additional request which can't really be avoided.

What I mean was, we could handle that connection part and the extra request part (for getting pubkey) in a separate crate and utilize the with_pubkey mode. I mean, we have an abstraction layer for external wallets, so why not using it?

we could handle that connection part and the extra request part (for getting pubkey) in a separate crate

how do you plan to do this? since, we either ways we still need to request for pubkey meanwhile with_pubkey params requires pubkey in activation params itself. There's a kdf_walletconnect crate that handles all WC logic By the way,.

WalletConnect coin activation mode lets the activation logic handle the session and fetch the key directly, setting it in TendermintActivationPolicy::PublicKey—keeping things simple without needing an abstraction layer.

Also, the current external wallet implementation for tendermint uses SSE, in the case of Keplr, we need to send request to GUI then to Keplr wallet if I'm correct, so it can't directly work with WalletConnect. The current activation logic is quite easy to understand and scale(when integrating KDF as Wallet in the future)

how do you plan to do this? since, we either ways we still need to request for pubkey meanwhile with_pubkey params requires pubkey in activation params itself. There's a kdf_walletconnect crate that handles all WC logic By the way,.

User sends the activation request for WC -> KDF takes the request and runs the logic where it reads Pubkey from WC -> Uses that Pubkey to run KDF as an external wallet (just like any other wallet).

Is that not possible?

Also, the current external wallet implementation for tendermint uses SSE, in the case of Keplr, we need to send request to GUI then to Keplr wallet if I'm correct, so it can't directly work with WalletConnect. The current activation logic is quite easy to understand and scale(when integrating KDF as Wallet in the future)

Yeah, for Keplr, GUI side triggers the Keplr events which when required. But it doesn't have to be the same way for WalletConnect.

User sends the activation request for WC -> KDF takes the request and runs the logic where it reads Pubkey from WC -> Uses that Pubkey to run KDF as an external wallet (just like any other wallet).

that's how the current flow works.. or am getting something wrong..
User request coin activation, logic checks if TendermintPubkeyActivationParams::WalletConnect( we can't use WithPubkey, since we don't have the pubkey at start)

komodo-defi-framework/mm2src/coins_activation/src/tendermint_with_assets_activation.rs

Lines 306 to 308 in f5caa19

TendermintPubkeyActivationParams::WalletConnect => {

activate_with_walletconnect(&ctx, protocol_conf.chain_id.as_ref(), &ticker).await?

},

then KDF takes the request and runs the logic where it reads Pubkey from WC( maybe this function should have a different naming)

komodo-defi-framework/mm2src/coins_activation/src/tendermint_with_assets_activation.rs

Lines 232 to 260 in f5caa19

async fn activate_with_walletconnect(

ctx: &MmArc,

chain_id: &str,

ticker: &str,

) -> MmResult<(TendermintActivationPolicy, TendermintWalletConnectionType), TendermintInitError> {

let wc = WalletConnectCtx::from_ctx(ctx).expect("WalletConnectCtx should be initialized by now!");

let account = cosmos_get_accounts_impl(&wc, chain_id)

.await

.mm_err(|err| TendermintInitError {

ticker: ticker.to_string(),

kind: TendermintInitErrorKind::UnableToFetchChainAccount(err.to_string()),

})?;

let wallet_type = if wc.is_ledger_connection().await {

TendermintWalletConnectionType::WcLedger

} else {

TendermintWalletConnectionType::Wc

};

let pubkey = match account.algo {

CosmosAccountAlgo::Secp256k1 | CosmosAccountAlgo::TendermintSecp256k1 => {

TendermintPublicKey::from_raw_secp256k1(&account.pubkey).ok_or(TendermintInitError {

ticker: ticker.to_string(),

kind: TendermintInitErrorKind::Internal("Invalid secp256k1 pubkey".to_owned()),

})?

},

};

Ok((TendermintActivationPolicy::with_public_key(pubkey), wallet_type))

}

Uses that Pubkey to run KDF as an external wallet (just like any other wallet).

komodo-defi-framework/mm2src/coins_activation/src/tendermint_with_assets_activation.rs

Line 259 in f5caa19

Ok((TendermintActivationPolicy::with_public_key(pubkey), wallet_type))

mm2src/kdf_walletconnect/src/chain.rs

mm2src/coins/tendermint/tendermint_token.rs

mm2src/coins/tendermint/tendermint_coin.rs

mariocynicys

Thanks for the PR! Very organized and easy to review.
Here is the first iteration.

mariocynicys · 2024-11-15T10:59:40Z

.cargo/config.toml

@@ -2,7 +2,7 @@
 JEMALLOC_SYS_WITH_MALLOC_CONF = "background_thread:true,narenas:1,tcache:false,dirty_decay_ms:0,muzzy_decay_ms:0,metadata_thp:auto"

 [target.'cfg(all())']
-rustflags = [ "-Zshare-generics=y" ]
+rustflags = [ "-Zshare-generics=y", '--cfg=curve25519_dalek_backend="fiat"' ]


Q: what is that for?

selecting the backend for compiling curve25519-dalek. This is explicitly needed because I updated the lib which now supports different backends for compilation

https://crates.io/crates/curve25519-dalek/4.1.3#simd-backend

mariocynicys · 2024-11-15T12:22:55Z

mm2src/coins/eth/v2_activation.rs

+    // #[serde(default)]
+    // pub use_walletconnect: Option<bool>,


mariocynicys · 2024-11-15T12:27:45Z

mm2src/coins/eth/v2_activation.rs

+            let public_key = compress_public_key(public_key_uncompressed)?;
+            Ok((
+                EthPrivKeyPolicy::WalletConnect {
+                    address,


this address doesn't seem to be used anywhere

mariocynicys · 2024-11-15T12:47:44Z

mm2src/coins/eth/wallet_connect.rs

+                .send_session_request_and_wait(&chain_id, WcRequestMethods::EthSignTransaction, tx_json, Ok)
+                .await?;
+            // First 4 bytes from WalletConnect represents Protoc info
+            hex::decode(&tx_hex[4..])?


is this the same as .strip_prefix(0x) in wc_send_tx? if so let's use the same for both.

Nop, this is some other WalletConnect protoc related data which is 4 bytes. stripprefix strips first 2 bytes I think

mariocynicys · 2024-11-15T12:50:37Z

mm2src/coins/eth/wallet_connect.rs

+        let tx_hash = tx_hash.strip_prefix("0x").unwrap_or(&tx_hash);
+        let maybe_signed_tx = {
+            // Wait for 10 seconds for the transaction to appear on the RPC node.
+            let wait_rpc_timeout = 10_000;


we should either set this as a const for visibility or mention in the doc comment that it waits for x amount of seconds.
also, the check_every seems very quick (1ms?)

mariocynicys · 2024-11-15T14:42:29Z

mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs

+        #[cfg(not(target_arch = "wasm32"))]
+        native_only_methods => match native_only_methods {
+            #[cfg(all(feature = "enable-solana", not(target_os = "ios"), not(target_os = "android")))]
+            "enable_solana_with_tokens" => {
+                handle_mmrpc(ctx, request, enable_platform_coin_with_tokens::<SolanaCoin>).await
+            },
+            #[cfg(all(feature = "enable-solana", not(target_os = "ios"), not(target_os = "android")))]
+            "enable_spl" => handle_mmrpc(ctx, request, enable_token::<SplToken>).await,
+            _ => MmError::err(DispatcherError::NoSuchMethod),
+        },


cruft from a mis-merge

mariocynicys · 2024-11-15T16:05:16Z

mm2src/coins/eth/wallet_connect.rs

+    Ok((uncompressed, recovered_address))
+}
+
+pub(crate) fn recover(signature: &Signature, message: &Message) -> Result<H520, ethkey::Error> {


let's use the Public type alias here for the Ok return type. one is not sure at first glance what this function recovers.

mariocynicys · 2024-11-15T16:19:08Z

mm2src/coins/utxo/utxo_common.rs

@@ -4744,6 +4748,7 @@ pub fn derive_htlc_key_pair(coin: &UtxoCoinFields, _swap_unique_data: &[u8]) ->
        PrivKeyPolicy::Trezor => todo!(),
        #[cfg(target_arch = "wasm32")]
        PrivKeyPolicy::Metamask(_) => panic!("`PrivKeyPolicy::Metamask` is not supported for UTXO coins"),
+        PrivKeyPolicy::WalletConnect { .. } => todo!(),


more descriptive panic would help here.
nit as of now: we might wanna refactor this method to not panic.

mariocynicys · 2024-11-15T18:12:01Z

mm2src/crypto/Cargo.toml

+[patch.crates-io]
+rand_core = { version = "0.6.2", package = "bip39" }


Q: can't understand this patch. why doesn't it have a url/git? could you explain how/what this does.

Yeah, I'm forcing bip39 to depend on rand_core 0.6.2 since the lib is selecting the least supported version(0.4.2) in kdf even tho the version isn't really compatible with the latest version of bip39
https://crates.io/crates/bip39/2.1.0/dependencies, I mean the feature of bip39 we using in kdf requires rand_core 0.6.2 or greater

The lib is configured to select any version from 0.4.2==0.7

mariocynicys · 2024-11-15T18:17:47Z

mm2src/mm2_main/src/rpc.rs

@@ -48,8 +48,9 @@ mod dispatcher_legacy;
 #[path = "rpc/lp_commands/lp_commands_legacy.rs"]
 pub mod lp_commands_legacy;
 mod rate_limiter;
+#[path = "rpc/wc_commands/wc_commands.rs"] pub mod wc_commands;


can we rename wc_commands.rs -> mod.rs and not use the path attribute here?

borngraced added 15 commits September 9, 2024 11:55

init, start walletconnect client impl

9100780

save dev state - walletconnect client impl

ca3f558

save dev state - WalletConnect client impl

c3dc6c6

rough implementations

0eea639

commit lock file

588316f

save dev state - handle connection close, session improvements, etc

60a2fc5

minor changes

fd8797c

fix wasm compilation

437767b

minor changes - create metadata mod

6374da3

minor changes and code organization

1c9cfe0

minor changes + merge session and session_key mod

f7987dd

minor changes to namespaces

1f2adb2

some improvements

658357f

add some session fn doc comments

98026ac

big improvements to session handling

9dddb91

borngraced self-assigned this Sep 16, 2024

borngraced added 14 commits September 17, 2024 11:27

implement session delete and cleaup

52a0f72

improve code and locking

9f5944c

remove unused dep

dff1b42

refactorings and cleanupds

591fbdb

minor changes

f6e7864

make session single

f535f0b

minor changes

3eb8969

save dev state

2d68488

implement coin activation for tendermint and other minor changes

42dd5ba

improve code and minor needed changes

edcb5a7

improve relayer disconnection, remove unneeded changes, track topic s…

e606ca4

…ubscriptions

fix tendermint pubkey derivation from walletconnect

d7b5a17

start persistent session storage implementation

3a85a17

implement sqlite session storage table and insert session method

be98c40

borngraced added 4 commits November 3, 2024 22:59

Merge branch 'refs/heads/dev' into wc-integration

fe0d096

# Conflicts: # mm2src/coins/Cargo.toml # mm2src/coins_activation/src/eth_with_token_activation.rs # mm2src/mm2_core/src/mm_ctx.rs

fix conflict and minor changes + merge with dev branch

a0220d7

minor changes

3468502

monitor if relay client is connected

c4a07e4

laruh reviewed Nov 5, 2024

View reviewed changes

mm2src/mm2_main/src/rpc.rs Show resolved Hide resolved

borngraced added 2 commits November 6, 2024 11:15

changes to client impls

1e78257

handle broadcast option for WalletConnect and some minor fixes

a43dc60

borngraced added under review and removed in progress Changes will be made from the author labels Nov 6, 2024

borngraced added 4 commits November 6, 2024 14:57

remove unused

eea1532

fix: handle unexpected end of file

fb9f8c1

re-enable use_watcher

d58b18f

cleanups

045625b

shamardy requested review from onur-ozkan and shamardy November 8, 2024 19:16

shamardy added the P2 label Nov 11, 2024

borngraced added 7 commits November 12, 2024 11:40

fix websocket connection drop

65c7f90

handle conn drop for session creation

2326959

nits

42eaa29

Merge branch 'refs/heads/dev' into wc-integration

4ad423e

# Conflicts: # Cargo.lock

fix conflict after merge

e82083a

nits

ad38241

move storage to session manager and minor improvements

59db165

onur-ozkan reviewed Nov 13, 2024

View reviewed changes

mm2src/coins/tendermint/tendermint_coin.rs Show resolved Hide resolved

mm2src/coins/tendermint/tendermint_coin.rs Outdated Show resolved Hide resolved

borngraced added 2 commits November 13, 2024 14:52

fix review notes

f5caa19

fix: self review

bb186c2

mariocynicys reviewed Nov 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(walletconnect): walletconnect integration #2223

feat(walletconnect): walletconnect integration #2223

borngraced commented Sep 16, 2024 •

edited

Loading

shamardy commented Nov 8, 2024

laruh commented Nov 12, 2024

onur-ozkan Nov 13, 2024

borngraced Nov 13, 2024 •

edited

Loading

onur-ozkan Nov 14, 2024

borngraced Nov 14, 2024

onur-ozkan Nov 14, 2024

borngraced Nov 14, 2024 •

edited

Loading

onur-ozkan Nov 14, 2024

borngraced Nov 14, 2024 •

edited

Loading

mariocynicys left a comment

mariocynicys Nov 15, 2024

borngraced Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

borngraced Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

mariocynicys Nov 15, 2024

borngraced Nov 15, 2024

mariocynicys Nov 15, 2024

	TendermintPubkeyActivationParams::WalletConnect => {
	activate_with_walletconnect(&ctx, protocol_conf.chain_id.as_ref(), &ticker).await?
	},

	async fn activate_with_walletconnect(
	ctx: &MmArc,
	chain_id: &str,
	ticker: &str,
	) -> MmResult<(TendermintActivationPolicy, TendermintWalletConnectionType), TendermintInitError> {
	let wc = WalletConnectCtx::from_ctx(ctx).expect("WalletConnectCtx should be initialized by now!");
	let account = cosmos_get_accounts_impl(&wc, chain_id)
	.await
	.mm_err(\|err\| TendermintInitError {
	ticker: ticker.to_string(),
	kind: TendermintInitErrorKind::UnableToFetchChainAccount(err.to_string()),
	})?;
	let wallet_type = if wc.is_ledger_connection().await {
	TendermintWalletConnectionType::WcLedger
	} else {
	TendermintWalletConnectionType::Wc
	};

	let pubkey = match account.algo {
	CosmosAccountAlgo::Secp256k1 \| CosmosAccountAlgo::TendermintSecp256k1 => {
	TendermintPublicKey::from_raw_secp256k1(&account.pubkey).ok_or(TendermintInitError {
	ticker: ticker.to_string(),
	kind: TendermintInitErrorKind::Internal("Invalid secp256k1 pubkey".to_owned()),
	})?
	},
	};

	Ok((TendermintActivationPolicy::with_public_key(pubkey), wallet_type))
	}

		[patch.crates-io]
		rand_core = { version = "0.6.2", package = "bip39" }

feat(walletconnect): walletconnect integration #2223

Are you sure you want to change the base?

feat(walletconnect): walletconnect integration #2223

Conversation

borngraced commented Sep 16, 2024 • edited Loading

How to test using EVM coin e.g ETH (Native && WASM)

shamardy commented Nov 8, 2024

laruh commented Nov 12, 2024

Choose a reason for hiding this comment

borngraced Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borngraced Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borngraced Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

mariocynicys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borngraced commented Sep 16, 2024 •

edited

Loading

How to test using EVM coin e.g `ETH` (Native && WASM)

borngraced Nov 13, 2024 •

edited

Loading

borngraced Nov 14, 2024 •

edited

Loading

borngraced Nov 14, 2024 •

edited

Loading